Skip to content

Conversation

@nicolasbeauvais
Copy link

In certain cases when using client or request SetTimeout it is possible that some trace value like dnsDone, tlsHandshakeDone or gotConn are zero, resulting in inaccurate results:

Screenshot From 2025-06-18 01-19-03
Screenshot From 2025-06-18 01-18-54
Screenshot From 2025-06-18 01-18-12

This PR add additional checks in the TraceInfo method to avoid negative values.

I'm not sure how to improve the tests for this, as aborting requests with SetTimeout will not always accurately stop the execution at the exact same point. I'm open to ideas.

Copy link
Member

@jeevatkm jeevatkm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolasbeauvais Thanks for the PR. I'm sorry for the delayed response, I was occupied with my personal stuff.

I added a one review comment.

@jeevatkm jeevatkm changed the title Fix negative trace substraction when using SetTimeout fix: negative trace substraction when using SetTimeout Nov 8, 2025
@jeevatkm
Copy link
Member

jeevatkm commented Nov 8, 2025

Also, the same issue might exist in the v2 too. I will backport it afterwards, or you can also submit PR v2 if you're interested.

@jeevatkm jeevatkm added this to the v3.0.0 Milestone milestone Nov 8, 2025
@jeevatkm jeevatkm added bug v3 For resty v3 labels Nov 8, 2025
@nicolasbeauvais
Copy link
Author

Hey @jeevatkm, no worries, I hope you're doing good!

I've set the default value directly at object initialization, let me know if that's ok for you. I can make a PR on the v2 branch once this one is approved.

Copy link
Member

@jeevatkm jeevatkm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolasbeauvais Thanks for updating PR.

@jeevatkm
Copy link
Member

@nicolasbeauvais It seems this test case having an issue TestTraceInfoOnTimeoutWithSetTimeout, can you please check it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug v3 For resty v3

Development

Successfully merging this pull request may close these issues.

2 participants